Skip to content

Tracing v1.0#2611

Open
IlaHadiAssar wants to merge 9 commits into
eclipse-ecal:masterfrom
IlaHadiAssar:feature/tracing/v1.0
Open

Tracing v1.0#2611
IlaHadiAssar wants to merge 9 commits into
eclipse-ecal:masterfrom
IlaHadiAssar:feature/tracing/v1.0

Conversation

@IlaHadiAssar
Copy link
Copy Markdown

This branch superseds the tracing poc

@IlaHadiAssar IlaHadiAssar force-pushed the feature/tracing/v1.0 branch from 06bd0d6 to 4185965 Compare April 17, 2026 10:48
@hannemn hannemn marked this pull request as ready for review May 7, 2026 08:16
@hannemn hannemn added the cherry-pick-to-NONE Don't cherry-pick these changes label May 7, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 20 out of 41. Check the log or trigger a new build to see more.

Comment thread ecal/core/include/ecal/config/tracing.h
std::shared_ptr<Logging::CLogProvider> g_log_provider_instance;
std::shared_ptr<Logging::CLogReceiver> g_log_receiver_instance;

std::shared_ptr<tracing::TraceProvider> g_trace_provider_instance;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'g_trace_provider_instance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

  std::shared_ptr<tracing::TraceProvider> g_trace_provider_instance;
                                          ^

Comment thread ecal/core/src/pubsub/ecal_publisher_impl.cpp
Comment thread ecal/core/src/pubsub/ecal_subscriber_impl.cpp
Comment thread ecal/core/src/pubsub/ecal_subscriber_impl.cpp
constexpr size_t kDefaultTracingBatchSize = 10;

// Specifies the type of operation being traced
enum operation_type
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: enum 'operation_type' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]

    enum operation_type
         ^

};

// Specifies the direction of the topic (publisher or subscriber)
enum topic_direction
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: enum 'topic_direction' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]

    enum topic_direction
         ^


// Bitmask enum for active transport layers used in tracing spans.
// These use power-of-two values so combinations can be expressed with bitwise OR.
enum eTracingLayerType : uint64_t
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: enum 'eTracingLayerType' uses a larger base type ('uint64_t' (aka 'unsigned long'), size: 8 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]

    enum eTracingLayerType : uint64_t
         ^

Comment thread ecal/core/src/tracing/tracing_writer.h
Comment thread ecal/core/src/tracing/tracing_writer.h Outdated
{

// Interface for tracing writers.
class TracingWriter
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: class 'TracingWriter' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

  class TracingWriter
        ^

IlaHadiAssar and others added 2 commits May 11, 2026 11:19
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Comment thread ecal/core/src/pubsub/ecal_publisher.cpp Outdated

bool CPublisher::Send(CPayloadWriter& payload_, long long time_)
{
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave this file untouched, this is not necessary for this PR

@@ -0,0 +1,68 @@
/* ========================= eCAL LICENSE =================================
*
* Copyright (C) 2016 - 2025 Continental Corporation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the proper Aumovio copyright header

@@ -0,0 +1,75 @@
/* ========================= eCAL LICENSE =================================
*
* Copyright (C) 2016 - 2025 Continental Corporation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the proper Aumovio copyright header

@@ -0,0 +1,39 @@
/* ========================= eCAL LICENSE =================================
*
* Copyright (C) 2016 - 2025 Continental Corporation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aumovio header

@@ -0,0 +1,51 @@
/* ========================= eCAL LICENSE =================================
*
* Copyright (C) 2016 - 2025 Continental Corporation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aumovio header


namespace eCAL
{
namespace tracing
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style and 2 spaces for indentation instead of 4


// record topic metadata for tracing
{
eCAL::tracing::STopicMetadata meta;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe assign this meta data only when there is a g_trace_provider, and take only the scope from the if-statement below


// record topic metadata for tracing
{
eCAL::tracing::STopicMetadata meta;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as in publisher

Comment thread ecal/core/src/tracing/span.h Outdated

namespace eCAL
{
namespace tracing
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: 2 indentation

Comment thread ecal/core/src/tracing/span.cpp Outdated

namespace eCAL
{
namespace tracing
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-pick-to-NONE Don't cherry-pick these changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants